Utils::Bottles::tag: ARM tag is arm64_big_sur #7942

sjackman · 2020-07-08T16:43:07Z

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes? Here's an example.
Have you successfully run brew style with your changes locally?
Have you successfully run brew tests with your changes locally?

Consistent with the Linux bottle tags, aarch64_linux, armv7_linux, and x86_64_linux.

See

    
           @tag ||= "#{ENV["HOMEBREW_PROCESSOR"]}_#{ENV["HOMEBREW_SYSTEM"]}".downcase.to_sym

sjackman · 2020-07-08T16:43:22Z

The canonical name of ARM64 is AArch64. See https://en.wikipedia.org/wiki/AArch64
Do you prefer

aarch64_big_sur
arm_big_sur
arm64_big_sur
si_big_sur

For reference, the bottle tags on Linux are

aarch64_linux
armv7_linux
x86_64_linux

claui · 2020-07-08T18:04:22Z

The canonical name of ARM64 is AArch64. See https://en.wikipedia.org/wiki/AArch64
Do you prefer
* `aarch64_big_sur`

* `arm_big_sur`

* `si_big_sur`

Yes I do

Bo98 · 2020-07-08T18:28:46Z

Just so we know what Apple call it: uname -p returns arm and uname -m returns arm64.

sjackman · 2020-07-08T18:56:17Z

brew/Library/Homebrew/brew.sh

Line 122 in b9e2b82

HOMEBREW_PROCESSOR="$(uname -p)"

On Intel

$ uname -p
i386
$ uname -m
x86_64
$ brew ruby -e 'p ENV["HOMEBREW_PROCESSOR"]'
"Intel"

On ARM

$ uname -p
arm
$ uname -m
arm64
$ brew ruby -e 'p ENV["HOMEBREW_PROCESSOR"]'
arm

Should HOMEBREW_PROCESSOR be arm or arm64 or aarch64?

claui · 2020-07-08T19:27:14Z

Should HOMEBREW_PROCESSOR be arm or arm64 or aarch64?

I’d prefer aarch64.
The arm prefix can be confused with the 32-bit ARM architecture, especially if you throw armv7_linux bottles into the mix.

sjackman · 2020-07-08T19:27:30Z

brew/Library/Homebrew/brew.sh

Line 122 in b9e2b82

HOMEBREW_PROCESSOR="$(uname -p)"

I feel like this line should be changed to

-HOMEBREW_PROCESSOR="$(uname -p)"
+HOMEBREW_PROCESSOR="$(uname -m)"

and HOMEBREW_PROCESSOR should be arm64.

SMillerDev · 2020-07-08T19:34:42Z

I think standardizing on aarch64 sounds good, as it's the name of the instruction set just like x86_64 is. I'd like very much for linux's armv7 to be called aarch32 though 😄

sjackman · 2020-07-08T19:43:39Z

uname -m reports armv7 rather than aarch32. 🤷‍♂️

SMillerDev · 2020-07-08T19:45:00Z

uname -m reports armv7 rather than aarch32. man_shrugging

But I really like this bikeshed to be purple, cmon.

Bo98 · 2020-07-08T19:49:46Z

HOMEBREW_PROCESSOR is what it is on Intel because of the user-agent. I think we need to separate those concerns here.

claui · 2020-07-08T20:20:23Z

In light of what @Bo98 wrote, I’d rather leave HOMEBREW_PROCESSOR as is.

We may want a separate environment variable for "$(uname -m)". Alternatively, how about we derive the tag from Hardware::CPU.arch?

sjackman · 2020-07-08T21:25:17Z

But I really like this bikeshed to be purple, cmon.

Sorry, we're all sold out of purple. 💜 😉

sjackman · 2020-07-08T21:32:14Z

In light of what @Bo98 wrote, I’d rather leave HOMEBREW_PROCESSOR as is.

HOMEBREW_PROCESSOR remains Intel on macOS and aarch64 or x86_64 on Linux. You prefer arm for macOS on Apple Silicon?

We may want a separate environment variable for "$(uname -m)". Alternatively, how about we derive the tag from Hardware::CPU.arch?

If it's done for Linux as well as macOS that'd change the bottle tag prefix
from armv6, aarch64, and x86_64
to arm, arm64, and x86_64.

Our naming is a bit inconsistent and crazy making now that I've jumped down this rabbit hole. 🕳️🐇

sjackman · 2020-07-08T21:35:23Z

I'd like to change HOMEBREW_PROCESSOR from Intel to x86_64 on macOS, to match Hardware::CPU.arch identically. We could rename HOMEBREW_PROCESSOR to HOMEBREW_CPU_ARCH while we're at it.

claui · 2020-07-08T21:48:32Z

HOMEBREW_PROCESSOR remains Intel on macOS and aarch64 or x86_64 on Linux. You prefer arm for macOS on Apple Silicon?

I don’t know semantics nor purpose of HOMEBREW_PROCESSOR so I’ll better leave that decision to people who do ☺️

If it's done for Linux as well as macOS that'd change the bottle tag prefix
from armv6, aarch64, and x86_64
to arm, arm64, and x86_64.

Oh I see. So that’s not what we want, either.

Bo98 · 2020-07-09T00:44:29Z

I'd like to change HOMEBREW_PROCESSOR from Intel to x86_64 on macOS, to match Hardware::CPU.arch identically. We could rename HOMEBREW_PROCESSOR to HOMEBREW_CPU_ARCH while we're at it.

I wouldn't be against this - but I'd make sure the user-agent still uses Intel, perhaps even unconditionally on the CPU for now (Google Analytics doesn't appear to support parsing what we're doing on ARM right now - I'm only seeing results for Intel).

mistydemeo · 2020-07-09T06:26:33Z

si_big_sur is clearly better. The text being unrenderable on Linux only emphasizes that this is a Mac-only architecture. /s

I'd like to change HOMEBREW_PROCESSOR from Intel to x86_64 on macOS

I'm -0 on this. It's not a major objection but I'd be concerned about the possibility of subtle bugs emerging, and I'm not sure if the benefits would outweight the potential of digging up weird bugs for awhile.

MikeMcQuaid · 2020-07-09T08:03:51Z

Library/Homebrew/extend/os/mac/utils/bottles.rb

+        if Hardware::CPU.intel?
+          MacOS.version.to_sym
+        else
+          "#{ENV["HOMEBREW_PROCESSOR"]}_#{MacOS.version}".to_sym


Just to keep up with the most recent conversation: what does this output on
Apple Silicon, currently? What does it output on Linux-on-ARM?

Apple Silicon (on master):

irb(main):001:0> Utils::Bottles.tag => :big_sur_arm irb(main):002:0> "#{ENV["HOMEBREW_PROCESSOR"]}_#{MacOS.version}".to_sym => :"arm_11.0"

Apple Silicon (on this PR branch):

irb(main):001:0> Utils::Bottles.tag => :"arm64_11.0" irb(main):002:0> "#{ENV["HOMEBREW_PROCESSOR"]}_#{MacOS.version}".to_sym => :"arm64_11.0"

Thanks! In that case:

Suggested change

"#{ENV["HOMEBREW_PROCESSOR"]}_#{MacOS.version}".to_sym

"#{ENV["HOMEBREW_PROCESSOR"]}_#{MacOS.version.to_sym}".to_sym

At least so we're not referring to it as 11.0 (which isn't even a "valid" symbol).

Does Apple refer to it as ARM64 much/at all?

Does Apple refer to it as ARM64 much/at all?

They’re going to great lengths to avoid any mention of ARM in their public-facing copy.

Given that I wonder about using one of the terms they actually use? Can you summarise them at all @claui?

Apple Silicon is the brand they use but when they talk about the architecture they use arm64 exclusively. No aarch64 even though it's the same thing.

e.g. https://developer.apple.com/documentation/xcode/porting_your_macos_apps_to_apple_silicon

Gotcha. I'd be fine with arm64_big_sur or apple_silicon_big_sur.

My reasoning for big_sur_arm was that we will likely end up having a 11.x where they/we don't support x86_64 any longer.

I prefer arm64 over apple_silicon, because it's what's returned by uname -m, and it's many characters shorter.

Ready for re-review.

MikeMcQuaid

Fine with me 👍

sjackman · 2020-07-10T17:54:17Z

We may want a separate environment variable for "$(uname -m)". Alternatively, how about we derive the tag from Hardware::CPU.arch?

If it's done for Linux as well as macOS that'd change the bottle tag prefix
from armv6, aarch64, and x86_64
to arm, arm64, and x86_64.

@claui I forget that I'm modifying extend/os/mac/utils/bottles.rb here, so this change applies only to macOS and doesn't affect Linux.

Utils::Bottles::tag: ARM tag is arm64_big_sur

sjackman · 2020-07-10T17:56:58Z

I've changed this PR to use Hardware::CPU.arch rather than HOMEBREW_PROCESSOR on macOS. Linux continues to use HOMEBREW_PROCESSOR.

MikeMcQuaid · 2020-07-10T18:11:43Z

Library/Homebrew/extend/os/mac/utils/bottles.rb

+        if Hardware::CPU.intel?
+          MacOS.version.to_sym
+        else
+          "#{Hardware::CPU.arch}_#{MacOS.version.to_sym}".to_sym


What's the expected string on Apple Silicon now?

brew/Library/Homebrew/hardware.rb

Lines 40 to 42 in 28f4a70

def arch_64_bit

if arm?

:arm64

arm64

sjackman · 2020-07-10T18:19:12Z

❌ codecov/patch — 66.67% of diff hit (target 73.06%)

https://codecov.io/gh/Homebrew/brew/compare/28f4a700acc1c5835f5565212a91b4541fcf469f...9a83e52ae479607084c6df198ec21a93d371cb7b/diff

😢 The ARM branch of this code isn't tested. Any suggestions?

MikeMcQuaid · 2020-07-10T19:00:17Z

😢 The ARM branch of this code isn't tested. Any suggestions?

That's fine, ignore it.

MikeMcQuaid

sjackman · 2020-07-10T23:33:25Z

Do we want to use x86_64_big_sur or big_sur for Big Sur on x86-64?

claui · 2020-07-10T23:34:07Z

Good job @sjackman 🍻

claui · 2020-07-11T08:38:05Z

Do we want to use x86_64_big_sur or big_sur for Big Sur on x86-64?

I’d be ok with leaving as is, given that x86_64 bottles will organically bubble up and go away in a few years.

sjackman self-assigned this Jul 8, 2020

sjackman requested review from MikeMcQuaid and mistydemeo July 8, 2020 16:43

sjackman mentioned this pull request Jul 8, 2020

os/mac/utils/bottles: support ARM bottles. #7815

Merged

6 tasks

sjackman requested a review from SMillerDev July 8, 2020 16:50

sjackman force-pushed the bottle_tag branch from cc5c790 to a0c9de7 Compare July 8, 2020 18:55

sjackman requested review from Bo98 and fxcoudert July 8, 2020 19:26

sjackman force-pushed the bottle_tag branch from a0c9de7 to 6b11fa4 Compare July 8, 2020 19:31

sjackman requested a review from claui July 8, 2020 19:31

sjackman force-pushed the bottle_tag branch from 6b11fa4 to 92dcfd0 Compare July 8, 2020 19:32

sjackman changed the title ~~Utils::Bottles::tag: ARM tag is arm_big_sur~~ Utils::Bottles::tag: ARM tag is arm64_big_sur Jul 8, 2020

MikeMcQuaid reviewed Jul 9, 2020

View reviewed changes

sjackman force-pushed the bottle_tag branch from 92dcfd0 to 3564b51 Compare July 9, 2020 19:44

MikeMcQuaid approved these changes Jul 10, 2020

View reviewed changes

Utils::Bottles::tag: ARM tag is arm_big_sur

e18027a

sjackman force-pushed the bottle_tag branch from 3564b51 to ec83f54 Compare July 10, 2020 17:55

HOMEBREW_PROCESSOR is arm64 on Apple Silicon

9a83e52

Utils::Bottles::tag: ARM tag is arm64_big_sur

sjackman force-pushed the bottle_tag branch from ec83f54 to 9a83e52 Compare July 10, 2020 17:56

MikeMcQuaid reviewed Jul 10, 2020

View reviewed changes

MikeMcQuaid approved these changes Jul 10, 2020

View reviewed changes

sjackman merged commit 8175e40 into Homebrew:master Jul 10, 2020

sjackman deleted the bottle_tag branch July 10, 2020 23:32

fxcoudert mentioned this pull request Nov 6, 2020

Fix test for ARM #9067

Merged

BrewTestBot added the outdated PR was locked due to age label Dec 24, 2020

Homebrew locked as resolved and limited conversation to collaborators Dec 24, 2020

	"#{ENV["HOMEBREW_PROCESSOR"]}_#{MacOS.version}".to_sym
	"#{ENV["HOMEBREW_PROCESSOR"]}_#{MacOS.version.to_sym}".to_sym

Utils::Bottles::tag: ARM tag is arm64_big_sur #7942

Utils::Bottles::tag: ARM tag is arm64_big_sur #7942

Conversation

sjackman commented Jul 8, 2020 • edited

sjackman commented Jul 8, 2020 • edited

claui commented Jul 8, 2020

Bo98 commented Jul 8, 2020

sjackman commented Jul 8, 2020

claui commented Jul 8, 2020

sjackman commented Jul 8, 2020

SMillerDev commented Jul 8, 2020

sjackman commented Jul 8, 2020

SMillerDev commented Jul 8, 2020

Bo98 commented Jul 8, 2020

claui commented Jul 8, 2020

sjackman commented Jul 8, 2020

sjackman commented Jul 8, 2020 • edited

sjackman commented Jul 8, 2020

claui commented Jul 8, 2020

Bo98 commented Jul 9, 2020 • edited

mistydemeo commented Jul 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeMcQuaid left a comment

Choose a reason for hiding this comment

sjackman commented Jul 10, 2020

sjackman commented Jul 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjackman commented Jul 10, 2020

MikeMcQuaid commented Jul 10, 2020

MikeMcQuaid left a comment

Choose a reason for hiding this comment

sjackman commented Jul 10, 2020 • edited

claui commented Jul 10, 2020

claui commented Jul 11, 2020

sjackman commented Jul 8, 2020 •

edited

sjackman commented Jul 8, 2020 •

edited

sjackman commented Jul 8, 2020 •

edited

Bo98 commented Jul 9, 2020 •

edited

sjackman commented Jul 10, 2020 •

edited